No new Suggestion-level findings this round — all prior suggestions have been addressed or superseded.

— qwen3.7-max via Qwen Code /review

✅ Hands-on verification (real binary) — merge reference

I built the real qwen bundle from this PR's head (60750480, base 47f62a46) and drove the feature end-to-end. Core was resolved from the PR's own source (an esbuild alias over the workspace symlink), and I confirmed the PR code is actually in the bundle (addMutationListener, Extensions changed on disk, reload-plugins all present in dist/chunks/). This complements the existing "no blockers" review — it answers does it actually behave as described?

Verdict: yes. All three user-facing paths work in the live TUI, the author's 341 tests pass, and the suite provably bears weight under mutation. No functional regression observed.

1) Real TUI — the actual qwen binary, isolated HOME, a demo extension, edits driven on disk

2) Tests — author's suites + a real-fs E2E + adversarial mutation

• 341 / 341 of the PR's own tests pass (core 195 + cli 146, 11 files).
• Real chokidar E2E (added for this review): the shipped suite mocks chokidar + node:fs and fires synthetic events, so I wrote a harness that drives the real watcher against a real temp dir with real edits. 9/9, deterministic across 4 runs — commands|skills|agents → content, hooks|qwen-extension.json|QWEN.md → stale, linked-extension source → content, .DS_Store|.swp|README.md → ignored.
• Mutation testing — 5/5 caught: dropping skills from AUTO_REFRESH_DIRS (RED in both unit and the real E2E), isSuppressed()→false, summary separator, throw hookReloadError, and dropping agent-scoped-hook preservation each turn the right suite RED and revert cleanly to green.
• --bare → no watcher is gated in source (gemini.tsx:621, isBareMode(argv.bare) ? undefined : new ExtensionFileWatcher(...)) and covered by the bare-mode test.

Notes (non-blocking, for the record)

• The one error path that is surfaced rather than swallowed — hook-reload failure (refreshExtensionRuntime re-throws hookReloadError) — is real and tested (mutation RED when the throw is removed). The remaining silent paths (ENOSPC, MCP startup, /hooks disable state across reload) are the author's explicitly-acknowledged out-of-scope items.
• On macOS, FSEvents occasionally emits an incidental sibling re-stat under rapid multi-file writes. It is an OS-level watcher trait (reproduced with bare chokidar, not specific to this PR) and is absorbed by awaitWriteFinish (200 ms) + the "prompt once" idempotency in ExtensionRefreshState. Worst case is one extra /reload-plugins prompt, never a wrong classification.
• By design, SUPPRESS_AFTER_MS = 1000 means a user file edit made within ~1 s after a programmatic enable/disable settles is intentionally ignored (anti-self-trigger). Sensible tradeoff; just noting it.

Bottom line: behavior matches the description, error surfacing on the hook path works, and the tests hold. LGTM to merge from a verification standpoint. 🚢

_{Verified locally on macOS with a from-source qwen bundle; screenshots are faithful renders of the real captured terminal output.}

@qwen-code /triage

Third triage pass on this PR. Since the last run (July 6), there's been a /review cycle, a maintainer's end-to-end tmux verification on Linux, and a merge conflict resolution. The code has matured significantly. Running through the gates again with the current state.

Template looks good ✓ — all required sections filled in, bilingual description, risk disclosure, test-plan steps with OS matrix.

Problem is real: extension developers currently have to type /reload-plugins after every file edit on disk. That's a friction point during rapid iteration, and it's referenced in #3696. Not theoretical.

Direction is aligned — auto-detecting extension file changes and hot-reloading content while prompting for config/hooks is exactly how developers expect a plugin system to behave. No product-direction concerns.

Size: core paths (packages/core/src/extension/, packages/core/src/hooks/) carry 383 production lines across 4 files — below the 500-line escalation threshold. Full PR is ~1,647 production lines (36 files including ~270 lines of i18n translation data and ~800 lines of tests), which triggers the non-blocking large-PR advisory. The scope is cohesive though — splitting would leave an incomplete feature.

Approach: chokidar file watcher with a three-way classification (auto-refresh for commands/skills/agents, stale-prompt for config/hooks, ignore for everything else) plus a shared ExtensionRefreshState event bus between watcher and UI is the right architecture. The mutation lifecycle pattern (paired start/end IDs) for watcher suppression during programmatic changes is a clean abstraction. Consolidating refreshMemory() + refreshTools() into one refreshExtensionRuntime() is a necessary cleanup to support the new code paths. I don't see a materially simpler path.

Moving to code review. 🔍

— Qwen Code · qwen3.7-max

Code Review

Independent proposal: I'd use chokidar to watch the user extension directory plus linked extension source directories. Classify each file change as either "content" (commands, skills, agents — safe to hot-reload silently) or "config" (manifest, hooks, context files — needs full /reload-plugins because it can affect the slash command list, hook registry, and cache coherently). Add a /reload-plugins command for the full rebuild. Extract the scattered refresh logic from ExtensionManager into one shared function. Use a small state object between watcher and UI for debouncing and notification. Suppress the watcher during programmatic mutations (install, enable, disable) to avoid false positives.

The PR's implementation matches this closely and goes further in good ways:

• The mutation lifecycle with paired IDs in a Map correctly handles nested mutations (install triggering enable internally) without relying on stack order. Better than a simple depth counter.
• Atomic cache replacement in refreshCache() — builds the new map first, swaps it in only after success. Eliminates the partial-state window the old code had.
• Generation guard on watcher events prevents stale async callbacks from operating on outdated state after restartWatching().
• Bootstrap watcher for the case where the extensions directory doesn't exist yet — watches the parent and restarts when the directory appears.
• HookRegistry.reloadConfiguredHooks() correctly snapshots agent-scoped entries, clears configured entries, reprocesses, restores enabled state, and rolls back on failure. This is the kind of careful implementation that prevents subtle bugs.
• ExtensionRefreshState has thoughtful edge-case handling: reload-in-progress deferral, post-reload re-emission of pending events, failure-state preservation for retry, suppression windows with depth tracking.
• The error contract in refreshExtensionRuntime() is explicit and tested: MCP failures propagate, LSP partial failures propagate, hooks surface after settle, skills/subagents/memory are best-effort.

The ExtensionRuntimeRefreshConfig API change (from getToolRegistry().restartMcpServers() to reinitializeMcpServers(getSettingsMcpServers())) aligns with the settings-change hot reload path. Tests cover the new contract.

One issue found: in updateExtension's finally block, fs.promises.rm(tempDir) can throw before endMutation() runs, leaving the watcher permanently suppressed for the session. The /review bot flagged this as a suggestion but it wasn't applied. Fix: wrap the rm in its own try/catch so endMutation() always fires:

} finally {
  if (tempDir) {
    try {
      await fs.promises.rm(tempDir, { recursive: true, force: true });
    } catch {
      // temp dir cleanup best-effort; endMutation must always run
    }
  }
  endMutation();
}

Unit Tests

357 tests across 11 suites, all passing. Coverage includes happy paths, failure modes (MCP fatal, LSP partial failure, hook rollback, cache failure preservation), lifecycle boundaries (mutation events around enable/disable/install/uninstall/update), state preservation (reload-in-progress deferral, post-reload re-emission), and content-refresh serialization with the 5-pass limit.

Test count grew from 341 → 357 across review iterations — the author added tests specifically for the edge cases reviewers flagged.

Real-Scenario Testing

Relying on maintainer verification (wenshao, July 8, on Linux with the real esbuild bundle in tmux). The maintainer drove all 9 test-plan steps with the actual CLI and reported:

A) /reload-plugins output:
   "Reloaded extensions: 1 extension · 1 command · 1 skill · 1 agent · 1 hook ·
    0 extension MCP servers · 0 extension LSP servers"
   — all 7 counts correct, singular/plural correct

B) Config edit (qwen-extension.json):
   → one-time prompt: "Extensions changed on disk. Run /reload-plugins to apply updates."
   — NOT silently applied ✅

C) Content edit (add commands/bye.toml while running):
   → /bye auto-appears in command popup within ~250ms, no prompt, no manual reload ✅

Mechanism proof + negative control:

• Watcher debug log confirms correct classification: commands/*.toml → action: 'auto' (silent hot-reload); qwen-extension.json → action: 'stale' (prompt).
• qwen --bare produces 0 EXTENSION_FILE_WATCHER log lines while other debug categories are active — the watcher is simply never constructed. Isolates ExtensionFileWatcher as the sole source of reload behavior (test-plan step 9 ✅).
• Revert-proof: reverting each core file to base (keeping the PR tests) fails exactly the new tests — hookRegistry.ts → 3 fail, extensionManager.ts → 7 fail, extension-runtime-refresh.ts → 8 fail. Tests meaningfully lock behavior.

No correctness issues found by the maintainer. Clean merge, load-bearing tests, behavior matches spec. 🐧 Linux status: ⚠️ → ✅.

I didn't duplicate this testing — the maintainer's verification is more authoritative than anything I could orchestrate in headless CI for this particular feature (it requires live filesystem mutations to extension directories with chokidar debounce timing).

— Qwen Code · qwen3.7-max

Stepping back to look at the whole picture.

This is a well-scoped feature that solves a real pain point. Extension developers iterating on skill files, commands, or agents shouldn't have to manually reload after every save — that's table stakes for a plugin system. The problem is documented in #3696 and the PR addresses it directly.

The implementation is careful where it needs to be. The three-way file classification (auto / stale / ignore) is the right split — content files are safe to hot-reload, but hooks and config changes need a coordinated full rebuild because they can affect command routing, security-sensitive hook execution, and cache coherency. The ExtensionRefreshState state machine handles the genuinely complex interactions between watcher events, debouncing, programmatic mutation suppression, reload-in-progress deferral, and failure preservation. The hook reload logic with agent-scoped preservation and rollback-on-failure is the kind of implementation that prevents the subtle bugs that plague hot-reload systems.

My independent proposal matched the PR's architecture closely. I don't see a simpler path that would solve this problem. The scope feels right — the refreshMemory / refreshTools consolidation isn't drive-by refactoring, it's necessary for the new code paths to have a single refresh entry point.

What gives me confidence: the maintainer's tmux verification is thorough — real TUI output, mechanism proof via debug logs, negative control with --bare, and revert-proof test locking. That's more authoritative than anything I could reproduce in headless CI for a feature that requires live filesystem mutations with chokidar debounce timing. The author has been responsive to feedback across 14 commits, progressively hardening the state machine, clarifying failure modes, and tightening mutation boundaries.

The one issue I flagged — fs.promises.rm in updateExtension's finally block potentially skipping endMutation() — is a real bug but low-severity. Temp dir cleanup failures are uncommon, the impact is watcher suppression until CLI restart, and the fix is trivial.

At 383 production lines in core paths and ~1,647 total production lines, this is a substantial contribution from a fork. But the code is cohesive, well-tested (357 tests), and the maintainer has already verified it end-to-end on Linux and recommends merge.

Approving. The fs.promises.rm fix can be addressed in a follow-up or before merge at the maintainer's discretion.

— Qwen Code · qwen3.7-max

@qwen-code /triage

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

@qwen-code /resolve

Qwen Code resolved the merge conflicts and pushed the branch update.

Merge Conflict Resolution Summary — PR #6347

Base branch

main

Merge performed

git merge origin/main into qwen-resolve/pr-6347

Conflicted file

packages/cli/src/ui/hooks/slashCommandProcessor.ts (1 conflict, lines 58–72)

Resolution

The conflict was in the import block at the top of the file. Both sides extended the same import { parseSlashCommand } from '../../utils/commands.js' line differently:

• HEAD (PR branch) added extension-reload imports: AppEvent, t, refreshExtensionContentRuntime, EXTENSION_RELOAD_FAILED_REASON, ExtensionRefreshState — all used later in the file for the extension hot-reload feature.
• origin/main expanded the commands.js import to also pull in parseStackedSlashCommands and MAX_STACKED_SKILLS — used later in the file for the stacked slash-skill invocations feature (feat(cli): support stacked slash-skill invocations #6361).

Resolution: Combined both sides. The parseSlashCommand import was kept in main's expanded multi-line form (adding parseStackedSlashCommands and MAX_STACKED_SKILLS), and all four PR-specific import statements (AppEvent, t, refreshExtensionContentRuntime, EXTENSION_RELOAD_FAILED_REASON/ExtensionRefreshState) were preserved immediately after.

Commit

6c38cae43 — merge: integrate origin/main into feat/extension-file-reload

Auto-merged files (no manual intervention needed)

• packages/cli/src/i18n/locales/en.js, fr.js, ja.js, zh-TW.js, zh.js
• packages/cli/src/ui/AppContainer.tsx
• packages/cli/src/ui/commands/types.ts
• packages/cli/src/ui/hooks/slashCommandProcessor.test.ts

@qwen-code /triage

Suggestions — commit ad2236476

Needs Human Review (low confidence):

• packages/cli/src/config/extension-refresh-state.ts + packages/cli/src/ui/commands/reload-plugins-command.ts + packages/cli/src/ui/AppContainer.tsx — Three sites create new ExtensionRefreshState() as a fallback. If any fires, the event bus disconnects silently. Consider adding a console.warn in each fallback.
• packages/cli/src/config/extension-refresh-state.ts:89 — markExtensionsReloadFailed has no reloadInProgress precondition guard. All current callers are correct, but the API could be hardened.

— qwen3.7-max via Qwen Code /review

✅ Verification report — extension file-watch / hot-reload

I verified this PR end-to-end by merging it into current origin/main, running the full new unit surface with real vitest, and then driving the real esbuild bundle in a tmux TTY on Linux. All the behaviors in the Reviewer Test Plan reproduce. Recommend merge.

• Merge base: origin/main @ 880b06ed4 + PR ad2236476 → merges cleanly (mergeable: MERGEABLE).
• Env: Linux, Node 22, real dist/cli.js bundle (v0.19.7), isolated QWEN_HOME, a fixture extension demo-ext with commands/, skills/, agents/, and a manifest hook.

1) Real TUI behavior (A, B, C)

2) Mechanism proof + negative control + unit suites

• Classification is correct at runtime (watcher debug log): editing commands/*.toml → action: 'auto' (silent hot-reload); editing qwen-extension.json → action: 'stale' (prompt). This is exactly the content-vs-config split the design intends.
• Negative control — qwen --bare: the same edits produce 0 EXTENSION_FILE_WATCHER log lines, while the debug file is otherwise full (50 lines across SKILL_MANAGER, FILE_COMMAND_LOADER, …). Logging is active — the watcher is simply never constructed under --bare. This isolates ExtensionFileWatcher as the sole cause of the reload behavior (PR test-plan step 9 ✅).
• Unit suites (real vitest on the merge result): 326/326 pass across the 10 new/changed files.
• Revert-proof (load-bearing): swapping each modified core file back to base (keeping the PR test) fails exactly the new tests —
  • hookRegistry.ts → 3 fail (reloadConfiguredHooks is not a function, incl. the error-recovery / agent-scope-preservation cases)
  • extensionManager.ts → 7 fail (addMutationListener is not a function + the atomic cache-swap test)
  • extension-runtime-refresh.ts → 8 fail (LSP/hook error surfacing + the getToolRegistry() → reinitializeMcpServers() API change)

Notes for the maintainer

• 🐧 Linux status can go from ⚠️ → ✅. The watcher, classification, auto-reload, prompt, and --bare gate all behave correctly on Linux via chokidar.
• Breaking-change claim is accurate: ExtensionManager.refreshMemory() has no remaining callers anywhere in packages/ — removal is safe.
• Intentional asymmetry worth documenting: once a config/hook change has set "refresh needed", subsequent content edits stop silently hot-reloading (markExtensionContentChanged bails while extensionRefreshNeeded is true) — they get folded into the next /reload-plugins. Correct, but non-obvious.
• Latency nuance: end-to-end content refresh is the 250 ms debounce plus chokidar awaitWriteFinish (200 ms stability) plus the refresh, so it's closer to ~450–500 ms in practice than the "~250 ms" in the description — cosmetic only.
• The PR's self-disclosed limitations (no user-visible ENOSPC notice, MCP start failures only logged during reload, /hooks disable-state lost across reload, watcher dies with the process) all hold and are reasonable for this scope.
• Linked extensions (test-plan step 7): covered by 3 dedicated watcher unit tests (linked-source watch roots, linked-source change → refresh, inactive-link exclusion). I could not drive extensions link at runtime here because it blocks on a workspace-trust prompt with no non-interactive flag — but the linked path shares the same classification/refresh pipeline I verified for user-dir extensions.

No correctness issues found. Clean merge, load-bearing tests, behavior matches the spec. 👍

_{Verified on Linux by driving the real bundle (merge of origin/main + this PR) in tmux; unit runs are real vitest on the merge result.}

@qwen-code /triage

macOS tmux verification addendum — hooks / MCP / LSP

I ran one extra hands-on pass for the runtime pieces that were not covered deeply in the earlier tmux report: extension hooks, extension MCP, and extension LSP.

• PR head tested: ad2236476
• Env: macOS, real qwen from this PR's source bundle, real tmux TTY, isolated HOME / QWEN_HOME
• Startup: node scripts/start.js --sandbox=false --experimental-lsp

Results

• /reload-plugins after extension runtime changes reported:

Reloaded extensions: 1 extension · 2 commands · 0 skills · 0 agents · 1 hook · 1 extension MCP server · 1 extension LSP server

• MCP: /mcp showed the extension server as connected:

Extension MCPs
  tmux-empty-mcp · ✓ connected

The MCP server log confirmed a real restart and JSON-RPC handshake/list flow: initialize, notifications/initialized, prompts/list, resources/list, tools/list.

• LSP: /lsp showed the extension LSP server as ready:

| Server | Command | Languages      | Status |
|--------|---------|----------------|--------|
| node   | node    | tmux-empty-lsp | READY  |

The LSP server log confirmed initialize, initialized, and workspace/didChangeWorkspaceFolders.

• Hooks: after editing the extension hook config and running /reload-plugins, submitting a normal prompt executed the UserPromptSubmit command hook before the model request:

UserPromptSubmit: hook evidence prompt

The following model call failed with Connection error. (cause: fetch failed) because this smoke test used a dummy local model endpoint. That is expected and happens after the hook execution path above.

Verdict: this extra tmux pass covers the reload path for hooks, extension MCP reconnect, and extension LSP initialization. No new correctness issue found from these runtime checks.

Adding the visual evidence that should have been included with my approval.

These are screenshots rendered from the real tmux pane/log captures used for the macOS verification pass.

Reload / command discovery

MCP / LSP

Hook execution / runtime logs